Fix pause container of a privileged pod unable to start due to /sys mount option mismatch#2760
Conversation
eefdbba to
a10d1a1
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an IsSandboxContainer flag to CreateContainerOptions and threads it through the Rego policy enforcer so that the framework policy can special-case a /sys rw mount on the CRI pod sandbox (pause) container when the policy allows any elevated containers.
Changes:
- New
IsSandboxContainerfield onCreateContainerOptions, populated fromc.isSandboxinHost.CreateContainerand forwarded into the Rego input. - New
mount_okrule inframework.regothat permits a sysfs rw/sysmount for the sandbox container when at least one candidate container hasallow_elevated. - Tests covering sandbox vs. non-sandbox, ro/rw combinations, and the privileged-request-denied case.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/securitypolicy/securitypolicyenforcer.go | Adds IsSandboxContainer field with doc comment. |
| pkg/securitypolicy/securitypolicyenforcer_rego.go | Plumbs the flag through V1 wrapper and into the Rego input map. |
| pkg/securitypolicy/framework.rego | New mount_ok rule for sandbox sysfs rw carve-out; minor whitespace cleanup. |
| pkg/securitypolicy/regopolicy_linux_test.go | New tests for sandbox sysfs carve-out behavior. |
| internal/guest/runtime/hcsv2/uvm.go | Switches to EnforceCreateContainerPolicyV2 and supplies IsSandboxContainer: c.isSandbox. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e931899 to
a49098b
Compare
|
@anmaxvl don't forget to merge |
…ount option mismatch Starting with v2, containerd mounts /sys as rw on the sandbox container when the pod is privileged (1fc497218 "Fix privileged container sysfs can't be rw because pod is ro by default") instead of ro. This means that the mount list for a privileged pause container no longer matches with just data.defaultMounts and will need a special case for sysfs. Alternative options were also considered - see the comment in framework.rego. This change in GCS is necessary even though this can be fixed via a policy change, because we need to maintain compatibility with existing policies. This PR converts EnforceCreateContainerPolicy in the LCOW GCS to EnforceCreateContainerPolicyV2, in order to use the CreateContainerOptions struct to pass an additional bool indicating whether the current container is a sandbox container. This does not allow additional capabilities etc for the sandbox container, only that sysfs can now be rw. Assisted-by: GitHub Copilot:claude-opus-4.7 Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
Assisted-by: GitHub Copilot:claude-opus-4.7 Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
…for sysfs rw special case Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
… container in the policy is elevated Assisted-by: GitHub Copilot copilot-review Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
18fb719 to
3f3657f
Compare
micromaomao
left a comment
There was a problem hiding this comment.
I've removed the
some c in candidate_containers
c.allow_elevated
clause since even when a policy does not allow any elevated containers, some future fragments might do, Noticed in testing, fix pushed.
Starting with v2, containerd mounts /sys as rw on the sandbox container when the
pod is privileged (1fc497218 "Fix privileged container sysfs can't be rw because
pod is ro by default") instead of ro. This means that the mount list for a
privileged pause container no longer matches with just data.defaultMounts and
will need a special case for sysfs. Alternative options were also considered -
see the comment in framework.rego.
This change in GCS is necessary even though this can be fixed via a policy
change, because we need to maintain compatibility with existing policies.
This PR converts EnforceCreateContainerPolicy in the LCOW GCS to
EnforceCreateContainerPolicyV2, in order to use the CreateContainerOptions
struct to pass an additional bool indicating whether the current container is a
sandbox container.
This does not allow additional capabilities etc for the sandbox container, only
that sysfs can now be rw.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Tingmao Wang tingmaowang@microsoft.com